Fix: get node path from Windows registry, clearer error messages#5233
Fix: get node path from Windows registry, clearer error messages#5233vlascik wants to merge 2 commits intoblock:mainfrom
Conversation
Signed-off-by: V. Lascik <vlascik@users.noreply.github.com>
Signed-off-by: V. Lascik <vlascik@users.noreply.github.com>
|
|
||
| // If this is a Stdio extension that uses npx, check for Node.js installation | ||
| #[cfg(target_os = "windows")] | ||
| use winreg::enums::{HKEY_LOCAL_MACHINE, KEY_READ}; |
There was a problem hiding this comment.
this looks great. does this mean we can delete the code in the API handler that tries to do the same thing? but only in the API so that would not work for the CLI I think?
There was a problem hiding this comment.
what code do you mean? All I can see are some shell scripts trying to install node.js again in a default directory, which is probably not going to work for people that already have node installed elsewhere anyway (the chances are their install will just be updated).
There was a problem hiding this comment.
I see what you mean now, there is a new PR that moved this code to crates/goose-server/src/routes/agent.rs now, is this what you meant?
There was a problem hiding this comment.
yeah sorry I moved this - do you want to do that or should I just copy your code over if that is faster
There was a problem hiding this comment.
@DOsinga just copy it, it will be faster.
|
/cc @jamadeo can we add this to your path magic or should it say separate |
|
Files moved around, so I did a new rebase of this PR at #5731 to make review/merge easier. |
2 participants
Summary
On Windows when adding
npxextensions, Goose tries to findnode.exeat default paths inProgram Files, and then fails for everyone that installednodeelsewhere. This PR tries to also find the path from the registry, which should mitigate the problem for people that already have node installed, but in a different folder.Also the error messages are made a bit clearer about what the problem is, so that users can try to rectify it on their own.
The node.js install script doesn't seem to quite work, but that's left for another PR.
Type of Change
Testing
Build on Windows
Related Issues
Relates to #4030